Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support driverlog argument in profiler CLI #897

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Support driverlog argument in profiler CLI #897

merged 2 commits into from
Apr 3, 2024

Conversation

amahussein
Copy link
Collaborator

Signed-off-by: Ahmed Hussein (amahussein) a@ahussein.me

Fixes #676

  • This code allows passing --driverlog argument to the spark_rapids CLI
  • The spark_rapids profiling cmd can accept none eventlogs
  • Added unit test to the spark_rapids that eventlogs can be empty

@amahussein
Support driver argument in profiler CLI
f5019be 
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>

Fixes NVIDIA#676

- This code allows passing `--driverlog` argument to the `spark_rapids`
CLI
- The spark_rapids profiling cmd can accept none eventlogs
- Added unit test to the spark_rapids that eventlogs can be empty
@amahussein amahussein added bug Something isn't working user_tools Scope the wrapper module running CSP, QualX, and reports (python) new-cli scope out future new-cli work for the next few months labels Apr 2, 2024
@amahussein amahussein self-assigned this Apr 2, 2024
@amahussein amahussein requested a review from nartal1 April 2, 2024 23:21
nartal1
nartal1 reviewed Apr 3, 2024
View reviewed changes
Copy link
Collaborator

@nartal1 nartal1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @amahussein ! Overall LGTM. I wanted to understand the output from user tools if only driverlog is provided in the commandline.

user_tools/src/spark_rapids_tools/cmdli/tools_cli.py Outdated Show resolved Hide resolved
@amahussein amahussein changed the title Support driver argument in profiler CLI Support driverlog argument in profiler CLI Apr 3, 2024
@amahussein
Fix typos in the python doc
1bd52f6 
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
@amahussein
Copy link
Collaborator Author

amahussein commented Apr 3, 2024 •
edited
Loading

Thanks @amahussein ! Overall LGTM. I wanted to understand the output from user tools if only driverlog is provided in the commandline.

So far, the user-tools does not consume the output from the driverlog.
IMHO, running the user-tools to process driverlog is more like an internal/engineering usage. It does not fit very well with the complete picture to serve the end-user.

Internally:

our devs can directly consume the CSV files and won't rely on the STDout and they will still be able to:

  • use it to verify accuracy of unsuported-operators.
  • evaluate a GPU app that does not meet the predicted speedup estimations.

From end-user perspective:

We probably need to integrate the output from the driverlog-processor to come-up with a meaningful summary. For example, we can integrate the output of driverlog into the AutoTuner as a way to give insights to the operators that caused any sort of fallbacks.
Once, this integration in place then the user-tools will be able to generate suitable report.

For followups:

We can file a separate issue to consume output of driverlog in the user-tools, but I believe it will be a redundant and low-priority task anyway. The other option is to take this into consideration while we are planning the revamp of AutoTuner running on top of both Qual/Prof tools.

@amahussein amahussein requested a review from nartal1 April 3, 2024 16:08
@nartal1
Copy link
Collaborator

nartal1 commented Apr 3, 2024

For followups:

We can file a separate issue to consume output of driverlog in the user-tools, but I believe it will be a redundant and low-priority task anyway. The other option is to take this into consideration while we are planning the revamp of AutoTuner running on top of both Qual/Prof tools.

Thanks @amahussein for the explanation. I think we can take this into consideration while we are planning the revamp of Autotuner. It might be good to explore if we can look into both driver-log output and metrics from the Prof tools for Autotuner recommendation.

@amahussein amahussein merged commit 3440285 into NVIDIA:dev Apr 3, 2024
15 checks passed
@amahussein amahussein deleted the spark-rapids-tools-676 branch April 3, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nartal1 nartal1 nartal1 approved these changes

Assignees

@amahussein amahussein

Labels
bug Something isn't working new-cli scope out future new-cli work for the next few months user_tools Scope the wrapper module running CSP, QualX, and reports (python)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Driver logs are currently not accepted by user tools
2 participants
@amahussein @nartal1